Skip to content

Add SafeAccess protection and frame type validation for signal handlers #349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
jbachorik merged 4 commits into from
Feb 4, 2026
Merged

Add SafeAccess protection and frame type validation for signal handlers #349

jbachorik merged 4 commits into from
Feb 4, 2026

Conversation

@jbachorik
Copy link
Collaborator

@jbachorik jbachorik commented Jan 30, 2026 •
edited
Loading

What does this PR do?:

This PR adds SafeAccess protection to several critical code paths in signal handler context and fixes invalid frame type handling:

  1. SafeAccess for frame pointer walking: Replaces raw pointer dereference with SafeAccess::load() in StackWalker::walkFP() to prevent ASan stack-buffer-overflow errors during frame pointer chain traversal.

  2. Frame type sanitization for J9: Adds validation and clamping for frame types to prevent invalid FrameTypeId values (e.g., value 48 causing ASAN errors). Implements defensive clamping in FrameType::decode() using FRAME_TYPE_MAX constant and properly handles negative BCI values (BCI_WALL, BCI_ERROR, etc.) through <= 0 check instead of == 0.

  3. SafeAccess for NativeFunc marking: Adds SafeAccess protection to NativeFunc::read_mark() which is called from signal handler context during stack walking.

  4. Test stability improvements: Fixes NPE and timing issues in flaky profiler tests, particularly in JfrDumpTest and BaseContextWallClockTest.

Motivation:

ASan detected multiple issues during signal handler execution:

  • Stack-buffer-overflow in walkFP() when traversing into stack frames with local variables
  • Invalid FrameTypeId values (e.g., 48) causing "not a valid value for type 'FrameTypeId'" errors
  • SEGV in NativeFunc::read_mark() when accessing potentially unmapped memory

The walkDwarf() function already uses SafeAccess::load() consistently (lines 190, 194), but walkFP() was inconsistent - using SafeAccess for PC but not for FP.

Additional Notes:

  • SafeAccess mechanism:
    • Uses assembly-level fault protection via safefetch64_impl / safefetch32_impl
    • If the read faults, the handler returns the default value (0/null), causing loops to exit naturally
    • Aligns walkFP() and read_mark() with the protection pattern used in walkDwarf()
  • Frame type clamping:
    • FRAME_TYPE_MAX constant provides future-proof clamping instead of hardcoded bit masks
    • Properly handles negative special BCI values through arithmetic right shift sign extension
  • Test infrastructure: run_docker_tests.sh can now target J9/OpenJ9 JVMs (8-j9, 11-j9, 17-j9, 21-j9)

How to test the change?:

Run ASan tests via CI (testAsan task).

The fixes prevent the following ASan errors that were occurring:

AddressSanitizer: stack-buffer-overflow in StackWalker::walkFP
READ of size 8 at address pointing to 'thread_count' (4 bytes)

runtime error: load of value 48, which is not a valid value for type 'FrameTypeId'

AddressSanitizer: SEGV in NativeFunc::read_mark(char const*)

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.

Unsure? Have a question? Request a review!

🤖 Generated with Claude Code

@jbachorik jbachorik added the AI label Jan 30, 2026
@dd-octo-sts
Copy link

dd-octo-sts bot commented Jan 30, 2026 •
edited
Loading

Scan-Build Report

User:runner@runnervmkj6or
Working Directory:/home/runner/work/java-profiler/java-profiler/ddprof-lib/src/test/make
Command Line:make -j4 all
Clang Version:Ubuntu clang version 18.1.3 (1ubuntu1)
Date:Tue Feb 3 17:18:25 2026

Bug Summary

Bug TypeQuantityDisplay?
All Bugs1
Unused code
Dead assignment1

Reports

Bug Group Bug Type ▾ File Function/Method Line Path Length
Unused codeDead assignmentlibraryPatcher_linux.cpppatch_library_unlocked941

@jbachorik jbachorik added the test:asan Run CI tests with AddressSanitizer configuration label Feb 2, 2026
@jbachorik jbachorik force-pushed the jb/fp_safeaccess branch 2 times, most recently from eedf923 to c394d1d Compare February 2, 2026 16:45
@jbachorik jbachorik changed the title Use SafeAccess for frame pointer dereference in walkFP WIP: Use SafeAccess for frame pointer dereference in walkFP Feb 2, 2026
@jbachorik jbachorik changed the title WIP: Use SafeAccess for frame pointer dereference in walkFP Use SafeAccess for frame pointer dereference in walkFP Feb 3, 2026
@jbachorik jbachorik changed the title Use SafeAccess for frame pointer dereference in walkFP Add SafeAccess protection and frame type validation for signal handlers Feb 3, 2026
@jbachorik jbachorik marked this pull request as ready for review February 3, 2026 09:39
@jbachorik jbachorik requested a review from a team as a code owner February 3, 2026 09:39
@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [x86_64 wall]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes wall wall
wall on on

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 14 metrics, 24 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [aarch64 wall]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes wall wall
wall on on

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 23 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [x86_64 memleak,alloc]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc on on
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak on on
modes memleak,alloc memleak,alloc
wall off off

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 23 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [x86_64 cpu]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu on on
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes cpu cpu
wall off off

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 23 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [aarch64 memleak,alloc]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc on on
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak on on
modes memleak,alloc memleak,alloc
wall off off

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 23 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [aarch64 alloc]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc on on
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes alloc alloc
wall off off

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 17 metrics, 21 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [aarch64 cpu,wall,alloc,memleak]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc on on
cpu on on
iterations 5 5
java "11.0.28" "11.0.28"
memleak on on
modes cpu,wall,alloc,memleak cpu,wall,alloc,memleak
wall on on

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 16 metrics, 22 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [aarch64 memleak]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak on on
modes memleak memleak
wall off off

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 23 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [aarch64 cpu,wall]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu on on
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes cpu,wall cpu,wall
wall on on

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 23 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [aarch64 cpu]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu on on
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes cpu cpu
wall off off

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 17 metrics, 21 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [x86_64 alloc]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc on on
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes alloc alloc
wall off off

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 23 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [x86_64 cpu,wall]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu on on
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes cpu,wall cpu,wall
wall on on

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 14 metrics, 24 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [x86_64 cpu,wall,alloc,memleak]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc on on
cpu on on
iterations 5 5
java "11.0.28" "11.0.28"
memleak on on
modes cpu,wall,alloc,memleak cpu,wall,alloc,memleak
wall on on

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 16 metrics, 22 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [x86_64 memleak]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak on on
modes memleak memleak
wall off off

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 23 unstable metrics.

jbachorik and others added 3 commits February 3, 2026 15:44
@jbachorik @claude
Use SafeAccess for frame pointer dereference in walkFP
91a04d4 
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@jbachorik @claude
Sanitize J9 frame types to prevent invalid FrameTypeId values
b0fc682 
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@jbachorik @claude
Use SafeAccess for NativeFunc::read_mark() in signal handler
fcdfdd1 
The read_mark() function can be called from signal handler context during
stack walking. Direct dereferencing of func->_mark can cause SEGV if the
computed pointer happens to be aligned but points to unmapped memory.

Use SafeAccess::safeFetch32() to safely read the mark field, extracting
the byte at offset 2 from the NativeFunc header.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Integration Tests

❔ 0 passed, 0 failed out of 0 configurations

Test Matrix

Platform JDK 8 JDK 11 JDK 17 JDK 21 JDK 25
glibc-x64-hotspot ❔ ❔ ❔ ❔ ❔
glibc-x64-openj9 ❔ ❔ ❔ ❔ ❔
glibc-arm64-hotspot ❔ ❔ ❔ ❔ ❔
glibc-arm64-openj9 ❔ ❔ ❔ ❔ ❔
musl-x64-hotspot ❔ ❔ ❔ ❔ ❔
musl-x64-openj9 ❔ ❔ ❔ ❔ ❔
musl-arm64-hotspot ❔ ❔ ❔ ❔ ❔
musl-arm64-openj9 ❔ ❔ ❔ ❔ ❔

Links

rkennke
rkennke approved these changes Feb 4, 2026
View reviewed changes
Copy link
Contributor

@rkennke rkennke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@jbachorik jbachorik merged commit a01b927 into main Feb 4, 2026
131 of 136 checks passed
@jbachorik jbachorik deleted the jb/fp_safeaccess branch February 4, 2026 14:12
@github-actions github-actions bot added this to the 1.38.0 milestone Feb 4, 2026
@jbachorik jbachorik mentioned this pull request Feb 10, 2026
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rkennke rkennke rkennke approved these changes

Assignees

No one assigned

Labels

AI test:asan Run CI tests with AddressSanitizer configuration

Projects

None yet

Milestone

1.38.0

Development

Successfully merging this pull request may close these issues.

2 participants

@jbachorik @rkennke